Online DDL: DROP TABLE translated to RENAME TABLE statement #7221

shlomi-noach · 2020-12-23T10:43:12Z

Description

Starting #7083, CREATE and DROP tables can execute as Online-DDL, meaning they're scheduled, audited and executed by tablet servers.

With this PR, a DROP TABLE statement is not only treated as online DDL, it is actually converted to a RENAME TABLE statement, to work with the table lifecycle mechanism.

The table is renamed into something like _vt_HOLD_b0d1fb34450a11ebb980f875a4d24e90_20201226103654. By just having that kind of name, the table is later garbage-collected by the lifecycle mechanism, to be purged->evacuated->dropped.

We take care of DROP TABLE IF EXISTS by checking the return code from the RENAME TABLE statement, and ignoring the specific error that indicates the table does not exist.

Also in this PR, CREATE and DROP are supported through vtgate. Previously they were only supported by vtctl.

Last, also in this PR, is retaining the GC UUID (b0d1fb34450a11ebb980f875a4d24e90 in the above _vt_HOLD_b0d1fb34450a11ebb980f875a4d24e90_20201226103654 example) such that:

The initial UUID is based on the Online DDL UUID. e.g.:

mysql> drop table tt;
+--------------------------------------+
| uuid                                 |
+--------------------------------------+
| b0d1fb34_450a_11eb_b980_f875a4d24e90 |
+--------------------------------------+

... time passes ...

mysql> show tables;
+----------------------------------------------------------+
| Tables_in_vt_commerce                                    |
+----------------------------------------------------------+
| _vt_HOLD_b0d1fb34450a11ebb980f875a4d24e90_20201226103654 |
| corder                                                   |
| customer                                                 |
| product                                                  |
+----------------------------------------------------------+

transitioning into next state, same UUID is reused, e.g. that table will transform into _vt_PURGE_b0d1fb34450a11ebb980f875a4d24e90_20201230103654
online ALTER TABLE artifacts are similarly named after the Online DDL UUID

There can be no conflict of naming because both state (HOLD, PURGE, etc.) as well as timestamps differ, even when using same UUID.

Related Issue(s)

tracking: #6689, #6926
Related: #7083
Depends on: #7151

Checklist

Should this PR be backported?
Tests were added or are not required
Documentation was added or is not required

Impacted Areas in Vitess

Components that this PR will affect:

Query Serving
VReplication
Cluster Management
Build

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

artifact tables are named after the migration UUID (this helps in tracking their origin) Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

shlomi-noach · 2020-12-23T11:22:57Z

Depends on: #7151

… DDL, otherwise consider them as direct regardless of ddl_strategy Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

shlomi-noach · 2020-12-23T14:28:39Z

Documentation PR to follow.

…o-rename Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

shlomi-noach · 2021-01-04T06:31:37Z

Request for review

rohit-nayak-ps

lgtm

go/vt/vttablet/onlineddl/executor.go

go/vt/schema/online_ddl_test.go

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

shlomi-noach · 2021-01-05T07:10:35Z

Merge conflicts are due to #7235
See proposed solution in #7247

systay · 2021-01-05T08:21:43Z

go/vt/schema/online_ddl.go

 	stmt, err := sqlparser.Parse(sql)
 	if err != nil {
-		return action, ddlStmt, fmt.Errorf("Error parsing statement: SQL=%s, error=%+v", sql, err)
+		return ddlStmt, action, fmt.Errorf("Error parsing statement: SQL=%s, error=%+v", sql, err)


:nit: :rant: This is confusing code. I know the old code did this, but we should fix it while we are in here. ddlStmt and action have not been assigned to here, so this is equivalent to

return nil, nil, fmt.Errorf("Error parsing statement: SQL=%s, error=%+v", sql, err)

I would suggest to not use named return arguments unless they help readability or need to be changed by a defer.

I personally find this to be more idiomatic, because I don't need to think about the actual datatype for those return variables and I don't need to think about what's the default return value -- it's implicitly done on my behalf. Also, to me, it more readable to see what's being returned semantically.

Having said that I don't feel strongly and I'll adapt per your review.

You are right to point out that confusing is highly subjective.

Let me explain where I'm coming from. I've always felt that it's important to return empty or nil values from a function when an error occurs. Some functions allow you to return both a valid value and an error (example: https://golang.org/src/io/io.go?s=11741:11795#L322), so I always make sure to explicitly return nil or empty values when the function doesn't have a valid value to return.

The named return argument pattern makes it a little more difficult for a casual reader to see if we are returning the result as far as we got when the error occurred, or an empty value to signal that nothing was actually done.

The way I see it, when an error is returned, in vast majority of cases the other values do not matter (are expected to be ignored). Therefore, it is only a rare occasion where those values will be checked. To me the code is more readable where I understand what it is that I'm returning semantically (e.g. this is the uuid rather than ""). And it's only the rare occasions where I need to worry about the actual values returned.

So I guess my confusion is the inversion of yours...

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

shlomi-noach · 2021-01-05T10:28:33Z

fixing failed tests after merging conflicts

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

shlomi-noach · 2021-01-05T13:31:16Z

endtoend tests fixed

shlomi-noach · 2021-01-05T13:31:26Z

to be followed by a documentation PR

shlomi-noach · 2021-01-24T09:17:04Z

documentation added and merged in vitessio/website#663

shlomi-noach added 5 commits December 22, 2020 16:26

Online DDL: convert DROP to RENAME statements

53e6d4a

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

an online DDL 'DROP TABLE' is now transformed into a 'RENAME TABLE'.

f7d564e

artifact tables are named after the migration UUID (this helps in tracking their origin) Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

vtgate: online DDL normalizes queries

1ac737c

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

merge master

30b9682

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

online DDL executor converts a DROP TABLE to RENAME TABLE statement

1eaaf5a

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

shlomi-noach mentioned this pull request Dec 23, 2020

Tracking issue: safe, lazy, managed DROP TABLE #6689

Open

shlomi-noach added 2 commits December 23, 2020 13:04

add tests from DROP TABLE, DROP TABLE IF EXISTS

f66b840

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

validating creation of GC table based on UUID

5dcf7e2

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

shlomi-noach changed the title ~~Online ddl drop to rename~~ Online DDL: DROP TABLE translated to RENAME TABLE statement Dec 23, 2020

shlomi-noach marked this pull request as ready for review December 23, 2020 11:22

shlomi-noach requested review from harshit-gangal, sougou and systay as code owners December 23, 2020 11:22

shlomi-noach added 2 commits December 23, 2020 13:38

tablet executor: only accept CREATE, DROP, ALTER statements as online…

c844a8d

… DDL, otherwise consider them as direct regardless of ddl_strategy Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

one more test

51d6042

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

shlomi-noach added 2 commits December 29, 2020 08:16

Merge remote-tracking branch 'upstream/master' into online-ddl-drop-t…

758fef4

…o-rename Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

if table is a GC table, just DROP it

45a5de7

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

rohit-nayak-ps self-requested a review January 4, 2021 16:41

rohit-nayak-ps approved these changes Jan 4, 2021

View reviewed changes

go/vt/vttablet/onlineddl/executor.go Outdated Show resolved Hide resolved

go/vt/schema/online_ddl_test.go Outdated Show resolved Hide resolved

shlomi-noach added 2 commits January 5, 2021 07:36

test UUID uniqueness

379e900

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

typo

59458fb

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

systay reviewed Jan 5, 2021

View reviewed changes

updates per review

ac4e855

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

systay approved these changes Jan 5, 2021

View reviewed changes

shlomi-noach added 2 commits January 5, 2021 11:31

Merge branch 'master' into online-ddl-drop-to-rename

16ccb98

test uuid

4428549

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

implementing GetIfExists for DropTable and DropView

ef06391

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

shlomi-noach merged commit 30b462a into vitessio:master Jan 5, 2021

shlomi-noach deleted the online-ddl-drop-to-rename branch January 5, 2021 13:37

askdba added this to the v9.0 milestone Jan 6, 2021

shlomi-noach mentioned this pull request Jan 7, 2021

Documenting new Online DDL behavior for DROP TABLE vitessio/website#663

Merged

rafael mentioned this pull request Mar 17, 2021

Slack vitess 9 2021.03.16r0 tinyspeck/vitess#200

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Online DDL: DROP TABLE translated to RENAME TABLE statement #7221

Online DDL: DROP TABLE translated to RENAME TABLE statement #7221

shlomi-noach commented Dec 23, 2020 •

edited

Loading

shlomi-noach commented Dec 23, 2020

shlomi-noach commented Dec 23, 2020

shlomi-noach commented Jan 4, 2021

rohit-nayak-ps left a comment

shlomi-noach commented Jan 5, 2021

systay Jan 5, 2021 •

edited

Loading

shlomi-noach Jan 5, 2021

shlomi-noach Jan 5, 2021

systay Jan 5, 2021

shlomi-noach Jan 5, 2021

shlomi-noach commented Jan 5, 2021

shlomi-noach commented Jan 5, 2021

shlomi-noach commented Jan 5, 2021

shlomi-noach commented Jan 24, 2021

Online DDL: DROP TABLE translated to RENAME TABLE statement #7221

Online DDL: DROP TABLE translated to RENAME TABLE statement #7221

Conversation

shlomi-noach commented Dec 23, 2020 • edited Loading

Description

Related Issue(s)

Checklist

Impacted Areas in Vitess

shlomi-noach commented Dec 23, 2020

shlomi-noach commented Dec 23, 2020

shlomi-noach commented Jan 4, 2021

rohit-nayak-ps left a comment

Choose a reason for hiding this comment

shlomi-noach commented Jan 5, 2021

systay Jan 5, 2021 • edited Loading

Choose a reason for hiding this comment

shlomi-noach Jan 5, 2021

Choose a reason for hiding this comment

shlomi-noach Jan 5, 2021

Choose a reason for hiding this comment

systay Jan 5, 2021

Choose a reason for hiding this comment

shlomi-noach Jan 5, 2021

Choose a reason for hiding this comment

shlomi-noach commented Jan 5, 2021

shlomi-noach commented Jan 5, 2021

shlomi-noach commented Jan 5, 2021

shlomi-noach commented Jan 24, 2021

shlomi-noach commented Dec 23, 2020 •

edited

Loading

systay Jan 5, 2021 •

edited

Loading